bitkeeper revision 1.1186 (420cff94zgt9FYAF2LFRO_Xztduveg)

Ensure coherency of guest-PDE 'accessed' bits when using shadow page
tables. There is no issue with 'dirty' bits because only PTEs and
super-page PDEs (which we do not support) have 'dirty' bits.
Signed-off-by: keir.fraser@cl.cam.ac.uk

diff --git a/xen/arch/x86/shadow.c b/xen/arch/x86/shadow.c
index 3652022666495f151c76a5e5be843a462030cc4a..5c9d43e028f0ed1cf26d6d3d796fbf6e8f24810a 100644 (file)
--- a/xen/arch/x86/shadow.c
+++ b/xen/arch/x86/shadow.c
@@ -11,12 +11,6 @@
 
 /********
 
-To use these shadow page tables, guests must not rely on the ACCESSED
-and DIRTY bits on L2 pte's being accurate -- they will typically all be set.
-
-I doubt this will break anything. (If guests want to use the va_update
-mechanism they've signed up for this anyhow...)
-
 There's a per-domain shadow table spin lock which works fine for SMP
 hosts. We don't have to worry about interrupts as no shadow operations
 happen in an interrupt context. It's probably not quite ready for SMP
@@ -484,8 +478,9 @@ unsigned long shadow_l2_table(
         spl2e = (l2_pgentry_t *)map_domain_mem(spfn << PAGE_SHIFT);
         /*
          * We could proactively fill in PDEs for pages that are already
-         * shadowed. However, we tried it and it didn't help performance.
-         * This is simpler.
+         * shadowed *and* where the guest PDE has _PAGE_ACCESSED set
+         * (restriction required for coherence of the accessed bit). However,
+         * we tried it and it didn't help performance. This is simpler. 
          */
         memset(spl2e, 0, DOMAIN_ENTRIES_PER_L2_PAGETABLE*sizeof(l2_pgentry_t));
 
@@ -498,7 +493,8 @@ unsigned long shadow_l2_table(
         spl2e[SH_LINEAR_PT_VIRT_START >> L2_PAGETABLE_SHIFT] =
             mk_l2_pgentry((spfn << PAGE_SHIFT) | __PAGE_HYPERVISOR);
         spl2e[PERDOMAIN_VIRT_START >> L2_PAGETABLE_SHIFT] =
-            mk_l2_pgentry(__pa(page_get_owner(&frame_table[gpfn])->arch.mm_perdomain_pt) |
+            mk_l2_pgentry(__pa(page_get_owner(
+                &frame_table[gpfn])->arch.mm_perdomain_pt) |
                           __PAGE_HYPERVISOR);
     }
 #endif
@@ -727,7 +723,7 @@ void shadow_l1_normal_pt_update(
 
 void shadow_l2_normal_pt_update(unsigned long pa, unsigned long gpde)
 {
-    unsigned long sl2mfn, spde;
+    unsigned long sl2mfn, spde = 0;
     l2_pgentry_t *spl2e;
     unsigned long sl1mfn;
 
@@ -736,11 +732,17 @@ void shadow_l2_normal_pt_update(unsigned long pa, unsigned long gpde)
 
     sl2mfn = __shadow_status(current->domain, pa >> PAGE_SHIFT) & PSH_pfn_mask;
 
-    sl1mfn = (gpde & _PAGE_PRESENT) ?
-        __shadow_status(current->domain, gpde >> PAGE_SHIFT) : 0;
+    /*
+     * Only propagate to shadow if _PAGE_ACCESSED is set in the guest.
+     * Otherwise, to ensure coherency, we blow away the existing shadow value.
+     */
+    if ( gpde & _PAGE_ACCESSED )
+    {
+        sl1mfn = (gpde & _PAGE_PRESENT) ?
+            __shadow_status(current->domain, gpde >> PAGE_SHIFT) : 0;
+        l2pde_general(current->domain, &gpde, &spde, sl1mfn);
+    }
 
-    /* XXXX Should mark guest pte as DIRTY and ACCESSED too! */
-    l2pde_general(current->domain, &gpde, &spde, sl1mfn);
     spl2e = (l2_pgentry_t *)map_domain_mem(sl2mfn << PAGE_SHIFT);
     spl2e[(pa & ~PAGE_MASK) / sizeof(l2_pgentry_t)] = mk_l2_pgentry(spde);
     unmap_domain_mem(spl2e);

diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index 62e30cd328a3bffb2ce78b8144a69f4d82a0ef82..073189c16acfc1425ac030da8365bdc146a7c6ca 100644 (file)
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -331,7 +331,7 @@ static inline void l2pde_general(
     {
         spde = (gpde & ~PAGE_MASK) | (sl1mfn << PAGE_SHIFT) | 
             _PAGE_RW | _PAGE_ACCESSED | _PAGE_DIRTY;
-        gpde |= _PAGE_ACCESSED | _PAGE_DIRTY;
+        gpde |= _PAGE_ACCESSED; /* N.B. PDEs do not have a dirty bit. */
 
         /* Detect linear p.t. mappings and write-protect them. */
         if ( (frame_table[sl1mfn].u.inuse.type_info & PGT_type_mask) ==